-
Notifications
You must be signed in to change notification settings - Fork 0
feat: select support groups #10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
WalkthroughIntroduces Select option grouping and loading state, adds a new SelectGroup component, updates Select internals (index, types, utils, styles) to support grouping and visuals, extends stories to demonstrate features, fixes misspelled megeRefs to mergeRefs in several components, and broadens react utils exports to export all from merge-refs. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App as App
participant Select as Select (Compound)
participant Group as SelectGroup
participant Utils as getChildrenData
participant UI as UI (Spinner/Menu)
App->>Select: Render with children (options, SelectGroup...)
Select->>Utils: Parse children (collect options, groupName)
Utils-->>Select: Structured data (ungrouped + grouped)
alt loading=true
Select->>UI: Show Spinner / disable interactions
else loading=false
Select->>UI: Render trigger (value, chevron)
Select->>Group: Render groups with labels
Select->>UI: Render ungrouped options first, then groups
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsonpackages/components/base/src/select/specs/__snapshots__/index.snapshot.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (10)
packages/components/base/src/boolean-input/BooleaInput.tsx(2 hunks)packages/components/base/src/dropdown/Dropdown.tsx(2 hunks)packages/components/base/src/file-uploader/FileUploader.tsx(2 hunks)packages/components/base/src/select/SelectGroup.tsx(1 hunks)packages/components/base/src/select/index.tsx(7 hunks)packages/components/base/src/select/styles/index.module.scss(4 hunks)packages/components/base/src/select/types.ts(5 hunks)packages/components/base/src/select/utils.ts(1 hunks)packages/docs/stories/src/form-select.stories.tsx(1 hunks)packages/utils/react/src/index.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
**/*.{ts,tsx}: Add TSDoc/TypeDoc comments to relevant entities
Only add examples if strictly needed
Keep descriptions not too long when possible, clear and objective to its purpose
Files:
packages/docs/stories/src/form-select.stories.tsxpackages/components/base/src/select/SelectGroup.tsxpackages/components/base/src/dropdown/Dropdown.tsxpackages/utils/react/src/index.tspackages/components/base/src/file-uploader/FileUploader.tsxpackages/components/base/src/boolean-input/BooleaInput.tsxpackages/components/base/src/select/index.tsxpackages/components/base/src/select/utils.tspackages/components/base/src/select/types.ts
packages/components/**
📄 CodeRabbit inference engine (.cursor/rules/building-components.mdc)
packages/components/**: Avoid importing or relying on external libraries when building or updating existing components
Reuse existing components in this codebase inside 'packages/components/' when building or updating components
Use foundations from 'packages/providers/' and 'packages/utils/**' when building or updating components
Files:
packages/components/base/src/select/SelectGroup.tsxpackages/components/base/src/dropdown/Dropdown.tsxpackages/components/base/src/select/styles/index.module.scsspackages/components/base/src/file-uploader/FileUploader.tsxpackages/components/base/src/boolean-input/BooleaInput.tsxpackages/components/base/src/select/index.tsxpackages/components/base/src/select/utils.tspackages/components/base/src/select/types.ts
🧬 Code graph analysis (7)
packages/components/base/src/select/SelectGroup.tsx (4)
packages/components/base/src/select/types.ts (1)
SelectGroupProps(112-117)packages/providers/manager/src/context.ts (1)
useManagerContext(12-12)packages/components/base/src/text/index.tsx (1)
Text(69-129)packages/utils/react/src/display-name.ts (1)
DISPLAY_NAME_ATTRIBUTE(3-3)
packages/components/base/src/dropdown/Dropdown.tsx (1)
packages/utils/react/src/merge-refs.ts (1)
mergeRefs(3-14)
packages/components/base/src/file-uploader/FileUploader.tsx (1)
packages/utils/react/src/merge-refs.ts (1)
mergeRefs(3-14)
packages/components/base/src/boolean-input/BooleaInput.tsx (1)
packages/utils/react/src/merge-refs.ts (1)
mergeRefs(3-14)
packages/components/base/src/select/index.tsx (3)
packages/components/base/src/select/utils.ts (1)
simplifyString(66-82)packages/components/base/src/spinner/index.tsx (1)
Spinner(21-32)packages/utils/react/src/merge-refs.ts (1)
mergeRefs(3-14)
packages/components/base/src/select/utils.ts (4)
packages/components/base/src/select/types.ts (5)
UserValue(7-7)SelectedValues(10-10)SelectChildrenData(123-140)SelectOptionProps(89-106)SelectGroupProps(112-117)packages/utils/react/src/children-without-fragments.ts (1)
getChildrenListWithoutFragments(6-19)packages/utils/react/src/display-name.ts (1)
getDisplayName(11-26)packages/utils/react/src/component-to-text.tsx (1)
componentToText(6-15)
packages/components/base/src/select/types.ts (1)
packages/components/base/src/select/index.tsx (1)
SelectGroupProps(431-431)
🔇 Additional comments (17)
packages/components/base/src/dropdown/Dropdown.tsx (1)
8-8: LGTM! Typo fix for mergeRefs.The misspelled
megeRefshas been corrected tomergeRefsin both the import and usage. The function is correctly imported from@react-ck/react-utilsand used to merge the forwarded ref with the internal containerRef.Also applies to: 144-144
packages/components/base/src/file-uploader/FileUploader.tsx (1)
7-7: LGTM! Typo fix for mergeRefs.The misspelled
megeRefshas been corrected tomergeRefsin both the import and usage. The function is correctly imported from@react-ck/react-utilsand used to merge the internal inputRef with the forwarded ref from inputProps.Also applies to: 109-109
packages/utils/react/src/index.ts (1)
11-11: LGTM! Removes the misspelled alias and exports all from merge-refs.The change from exporting specific named exports to
export * from "./merge-refs"removes the misspelledmegeRefsalias and provides a cleaner export surface. This aligns with the typo fixes in consuming components.packages/components/base/src/boolean-input/BooleaInput.tsx (1)
6-6: LGTM! Typo fix for mergeRefs.The misspelled
megeRefshas been corrected tomergeRefsin both the import and usage. The function is correctly imported from@react-ck/react-utilsand used to merge the forwarded ref with the internal inputRef.Also applies to: 61-61
packages/docs/stories/src/form-select.stories.tsx (4)
107-113: LGTM! Loading story demonstrates the new loading state.The story correctly demonstrates the Select component's loading state by setting
loading: true. The addition offullWidth: trueis appropriate for visual demonstration.
115-140: LGTM! WithGroups story demonstrates the new SelectGroup feature.The story correctly demonstrates the Select component's new grouping feature by using
Select.Groupwith thenameprop. The structure is clear and showcases three distinct groups with multiple options each.
142-154: LGTM! WithGroupsAndSearch story demonstrates groups with search.The story correctly combines the grouping feature with search functionality. The custom
emptyStateMessageprovides good UX when no results are found.
156-162: LGTM! WithGroupsMultiple story demonstrates groups with multiple selection.The story correctly combines the grouping feature with search and multiple selection. The updated placeholder text is appropriate for the multi-select context.
packages/components/base/src/select/SelectGroup.tsx (1)
29-47: LGTM! SelectGroup implementation is accessible and well-structured.The component correctly implements an accessible option group with proper ARIA attributes (
role="group"andaria-labelledby). The use ofgenerateUniqueIdfrom the manager context ensures unique IDs for the aria-labelledby relationship. The group name is rendered with appropriate text styling.packages/components/base/src/select/types.ts (3)
73-76: LGTM! Loading prop addition is straightforward.The
loadingprop is properly typed and documented with a clear description and default value. This aligns with the new loading state feature demonstrated in the stories.
108-117: LGTM! SelectGroupProps interface is well-defined.The interface correctly defines the props for the SelectGroup component with a required
namefor the label and optionalchildrenfor the options. The documentation is clear and follows the coding guidelines.
128-129: LGTM! SelectChildrenData extensions support grouping.The additions of
selectGroupPropsandgroupNamefields toSelectChildrenDatacorrectly support the new grouping feature, allowing options to be associated with their parent groups.Also applies to: 138-139
packages/components/base/src/select/styles/index.module.scss (5)
9-11: LGTM! Root positioning and cursor changes are appropriate.The addition of
position: relativeenables absolute positioning of child elements, andcursor: pointerprovides the correct visual feedback for the clickable select element.
20-26: LGTM! Value slot flex layout supports the new structure.The flex layout with
space-between,centeralignment, and gap provides proper spacing and alignment for the select value display, supporting the enhanced value rendering with groups and multiple selections.
36-43: LGTM! Display value item styling handles text overflow correctly.The inline-block display with vertical alignment and text truncation properties ensures that long option values are handled gracefully with ellipsis when they exceed the available width.
79-85: LGTM! Native element layering and loading state styling are correct.The
z-indexandpointer-events: noneon the native element ensure proper layering and prevent direct interaction with the hidden select. The.loadingclass appropriately disables pointer events during the loading state.
103-115: LGTM! Group styling provides clear visual separation.The
.groupand.group_namestyles correctly implement visual grouping with appropriate padding, borders, and margins. The use of:not(:first-of-type)ensures that only subsequent groups have top borders, creating clear separation between groups.
| const groupedOptions: { [key: string]: React.ReactNode[] } = {}; | ||
| const ungroupedOptions: React.ReactNode[] = []; | ||
|
|
||
| filtered.forEach((item) => { | ||
| if (item.isSelectOption) { | ||
| if (item.groupName) { | ||
| if (!(item.groupName in groupedOptions)) { | ||
| groupedOptions[item.groupName] = []; | ||
| } | ||
| groupedOptions[item.groupName]?.push(item.element); | ||
| } else { | ||
| ungroupedOptions.push(item.element); | ||
| } | ||
| } | ||
| }); | ||
|
|
||
| // Render grouped options | ||
| const result: React.ReactNode[] = []; | ||
|
|
||
| // Add ungrouped options first | ||
| ungroupedOptions.forEach((option) => result.push(option)); | ||
|
|
||
| // Add grouped options using SelectGroup component | ||
| Object.entries(groupedOptions).forEach(([groupName, options]) => { | ||
| result.push( | ||
| <SelectGroup key={groupName} name={groupName}> | ||
| {options} | ||
| </SelectGroup>, | ||
| ); | ||
| }); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Preserve SelectGroup props when rebuilding grouped options
We rebuild each group but only pass name, dropping every other prop coming from the original <Select.Group> (e.g. description, disabled, className, test ids). Any consumer relying on those props loses functionality as soon as a search term is applied. Please carry the captured selectGroupProps forward when rendering the grouped result so groups behave identically before and after filtering.
Suggested approach:
- const groupedOptions: { [key: string]: React.ReactNode[] } = {};
+ const groupedOptions: {
+ [key: string]: { options: React.ReactNode[]; props: SelectGroupProps };
+ } = {};
@@
- if (!(item.groupName in groupedOptions)) {
- groupedOptions[item.groupName] = [];
- }
- groupedOptions[item.groupName]?.push(item.element);
+ if (!(item.groupName in groupedOptions)) {
+ groupedOptions[item.groupName] = {
+ options: [],
+ props: item.selectGroupProps ?? { name: item.groupName },
+ };
+ }
+ groupedOptions[item.groupName]?.options.push(item.element);
@@
- Object.entries(groupedOptions).forEach(([groupName, options]) => {
+ Object.entries(groupedOptions).forEach(([groupName, { options, props }]) => {
result.push(
- <SelectGroup key={groupName} name={groupName}>
+ <SelectGroup key={groupName} {...props}>
{options}
</SelectGroup>,
);
});Make sure to import SelectGroupProps (or reuse the existing type) accordingly.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const groupedOptions: { [key: string]: React.ReactNode[] } = {}; | |
| const ungroupedOptions: React.ReactNode[] = []; | |
| filtered.forEach((item) => { | |
| if (item.isSelectOption) { | |
| if (item.groupName) { | |
| if (!(item.groupName in groupedOptions)) { | |
| groupedOptions[item.groupName] = []; | |
| } | |
| groupedOptions[item.groupName]?.push(item.element); | |
| } else { | |
| ungroupedOptions.push(item.element); | |
| } | |
| } | |
| }); | |
| // Render grouped options | |
| const result: React.ReactNode[] = []; | |
| // Add ungrouped options first | |
| ungroupedOptions.forEach((option) => result.push(option)); | |
| // Add grouped options using SelectGroup component | |
| Object.entries(groupedOptions).forEach(([groupName, options]) => { | |
| result.push( | |
| <SelectGroup key={groupName} name={groupName}> | |
| {options} | |
| </SelectGroup>, | |
| ); | |
| }); | |
| const groupedOptions: { | |
| [key: string]: { options: React.ReactNode[]; props: SelectGroupProps }; | |
| } = {}; | |
| const ungroupedOptions: React.ReactNode[] = []; | |
| filtered.forEach((item) => { | |
| if (item.isSelectOption) { | |
| if (item.groupName) { | |
| if (!(item.groupName in groupedOptions)) { | |
| groupedOptions[item.groupName] = { | |
| options: [], | |
| props: item.selectGroupProps ?? { name: item.groupName }, | |
| }; | |
| } | |
| groupedOptions[item.groupName]?.options.push(item.element); | |
| } else { | |
| ungroupedOptions.push(item.element); | |
| } | |
| } | |
| }); | |
| // Render grouped options | |
| const result: React.ReactNode[] = []; | |
| // Add ungrouped options first | |
| ungroupedOptions.forEach((option) => result.push(option)); | |
| // Add grouped options using SelectGroup component | |
| Object.entries(groupedOptions).forEach(([groupName, { options, props }]) => { | |
| result.push( | |
| <SelectGroup key={groupName} {...props}> | |
| {options} | |
| </SelectGroup>, | |
| ); | |
| }); |
🤖 Prompt for AI Agents
In packages/components/base/src/select/index.tsx around lines 121 to 151, the
code rebuilds grouped options but only passes the group `name` into the
recreated <SelectGroup>, dropping other props (description, disabled, className,
test ids, etc.). Modify the grouping logic to capture the original SelectGroup
props (use or import SelectGroupProps) alongside each group's elements when
building groupedOptions, store those props keyed by groupName, and when
rendering call <SelectGroup key={groupName} name={groupName}
{...selectGroupProps[groupName]}>{options}</SelectGroup> so all original group
props are preserved; ensure types reflect SelectGroupProps and maintain unique
keys as before.
| } else if (getDisplayName(i) === "SelectGroup" && React.isValidElement<SelectGroupProps>(i)) { | ||
| const { name, children: groupChildren } = i.props; | ||
| processChildren(groupChildren, name); | ||
| } else { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Group metadata is discarded when parsing children
When we hit a SelectGroup, we recurse into its children but never push an entry for the group itself, leaving selectGroupProps perpetually undefined. Downstream consumers (e.g., grouped rendering, search filtering) therefore lose access to the group wrapper, its props, and its element. Please push a SelectChildrenData entry containing the group element and its props (or otherwise expose the metadata) before recursing so callers can faithfully rebuild the grouping.
🤖 Prompt for AI Agents
In packages/components/base/src/select/utils.ts around lines 44 to 47, when
encountering a SelectGroup the code recurses into its children but does not
record the group itself; modify the branch to push a SelectChildrenData entry
for the group (including the group element and its props such as name and any
other group props) into the collection before calling
processChildren(groupChildren, name) so downstream code can reconstruct grouping
and access group metadata.
Summary by CodeRabbit
New Features
Bug Fixes
Style
Documentation
Refactor